Skip to content
This repository was archived by the owner on Jul 16, 2020. It is now read-only.

fixed error messages of JSON parsing is failed#12

Open
ivan-demchenko wants to merge 4 commits intoAPIs-guru:masterfrom
ivan-demchenko:fix/better-error-messages
Open

fixed error messages of JSON parsing is failed#12
ivan-demchenko wants to merge 4 commits intoAPIs-guru:masterfrom
ivan-demchenko:fix/better-error-messages

Conversation

@ivan-demchenko
Copy link

@ivan-demchenko ivan-demchenko commented Mar 8, 2018

I stumbled upon a nasty behaviour. I had an issue with one of the 100+ schemas is my RAML and it took me a few hours to debug the code and find out that the reason was a missing comma.

Now, if JSON parsing has failed, it will show the place where the issue is:

Unexpected string in JSON at position 16
{"some": "json" "that-is": "wrong"}
---------------^

@IvanGoncharov
Copy link
Member

IvanGoncharov commented Mar 8, 2018

@raqystyle Thanks for PR, cool feature 👍
I think it's valueable beyound this library.
So it would be great if you release it as NPM package.

I would merge it either way, just one suggestion it would be better if you just wrap JSON.parse.

@ivan-demchenko
Copy link
Author

ivan-demchenko commented Mar 9, 2018 •
edited
Loading

@IvanGoncharov I had the same idea of a separate npm package and already looking into differences between platforms ;)

As to your suggestion, I hope I got your idea correctly ;)

UPD: the PR's been updated

@ivan-demchenko ivan-demchenko force-pushed the fix/better-error-messages branch from ab8ff7e to 19d9c53 Compare March 9, 2018 00:53
@IvanGoncharov
Copy link
Member

IvanGoncharov commented Mar 9, 2018

@raqystyle Looks good for me 👍
@MikeRalphson What do you think?

@MikeRalphson
Copy link

MikeRalphson commented Mar 9, 2018

Should we allow the optional reviver parameter to be passed through to maintain compatibility?

@ivan-demchenko
Copy link
Author

ivan-demchenko commented Mar 13, 2018

@MikeRalphson I agree. I've started on this little lib, the core idea of which is to just provide a better error message.

@ivan-demchenko
Copy link
Author

ivan-demchenko commented Mar 23, 2018

@IvanGoncharov @MikeRalphson I've updated the PR ;)

@ivan-demchenko
Copy link
Author

ivan-demchenko commented Apr 10, 2018

@MikeRalphson this is a very good point. Fixed ;)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ivan-demchenko @IvanGoncharov @MikeRalphson @raqystyle